test: add Docker Compose healthcheck audit and enforcement#376
test: add Docker Compose healthcheck audit and enforcement#376bugman-007 wants to merge 3 commits intoLight-Heart-Labs:mainfrom
Conversation
Lightheartdevs
left a comment
There was a problem hiding this comment.
Review: Compose Healthcheck Audit
Script is well-structured and categorization logic is solid. A few issues to fix.
CLAUDE.md violation: 2>/dev/null
audit-compose-healthchecks.sh lines 50, 72, 77 — find and grep suppress stderr. CLAUDE.md rule 4 forbids this. Let errors surface or use explicit handling.
CLAUDE.md violation: || true in test
test-compose-healthcheck-audit.sh line 109 — Test #9 uses if [[ -x ... ]] || true; then pass. Always passes. Dead test. Violates both the || true ban and "let assertions fail visibly" rule.
Tests are structural, not behavioral
All 9 tests run the audit against the real repo and check for expected strings. None create temp compose files to verify detection/classification behavior. One fixture-based test with a known compose file (with and without healthcheck) would make these genuinely behavioral.
Bug: \s not portable to macOS
Line 72: grep -q "^services:\s*{}\s*$" uses \s which is PCRE, not supported in macOS grep's basic regex. CLAUDE.md calls out POSIX compatibility. Use [[:space:]] or grep -E.
CI conflict
PRs #373, #375, and #376 all insert CI steps at the same location in test-linux.yml. Whichever merges first conflicts with the others.
|
What's needed to get this merged:
Will conflict with #373 and #375 on |
|
I addressed review feedback for compose healthcheck audit |
Lightheartdevs
left a comment
There was a problem hiding this comment.
Review — Has CHANGES_REQUESTED status already. Notes for the resubmission.
Good additions:
- New
scripts/audit-compose-healthchecks.sh— scans all compose files for missing healthcheck definitions - Categorizes files (production vs local vs stub), supports
--strictand--quietflags - Test suite with behavioral tests using temp compose files
The grep -q "healthcheck:" approach is a reasonable heuristic but could miss healthchecks in overlay files (e.g., a service's healthcheck might be in compose.nvidia.yaml not compose.yaml). Consider noting this limitation.
Address existing review feedback and this is ready.
2da38f3 to
afd067b
Compare
|
All review feedback addressed:
Ready for re-review. |
afd067b to
053511d
Compare
053511d to
b3ecc16
Compare
|
Conflicts resolved Rebased on latest upstream main (commit 28da306). All merge conflicts have been resolved. The branch is now up to date and ready for re-review. |
|
Closing as part of v2.1.0 stabilization. We're pausing new feature PRs while we solidify the installer and CI gates. If this is still relevant for v2.2.0 (targeting April 1), please rebase and resubmit. Thank you for contributing. |
Summary
scripts/audit-compose-healthchecks.shwith categorization and strict modetests/test-compose-healthcheck-audit.shwith 9 test casesMotivation
Missing healthchecks = degraded observability
Current state:
docker-compose.base.ymlall have healthchecks ✅health-check.shrelies on healthchecks for monitoring ✅This PR provides visibility and a foundation for enforcement.
Audit Results
Scanned 32 compose files across the repository:
Files with healthchecks: 16
❌ Production files without healthchecks: 8
docker-compose.intel.yml- Intel Arc GPU overlaydocker-compose.arc.yml- Intel Arc GPU overlaydocker-compose.amd.yml- AMD GPU overlaydocker-compose.nvidia.yml- NVIDIA GPU overlaydocker-compose.apple.yml- Apple Silicon overlaydocker-compose.tier0.yml- Tier 0 configurationinstallers/windows/docker-compose.windows-amd.yml- Windows installerextensions/services/whisper/compose.nvidia.yaml- Whisper NVIDIA variantdocker-compose.local.ymlcompose.local.yamlfiles in extensionsℹ️ Stub files: 1
extensions/services/comfyui/compose.yaml(empty stub, GPU overlays have healthchecks)Tool Features
scripts/audit-compose-healthchecks.sh
Categorization:
*.local.*files (warnings only)services: {}(informational)Test Coverage
The test suite validates:
--strictflag enforcement works--quietflag reduces outputTest Results
CI Integration
Added "Compose Healthcheck Audit Tests" step to
.github/workflows/test-linux.ymlto run on every PR.Note: Currently runs in audit mode (non-blocking). Can be switched to
--strictmode in future PR once healthchecks are added to identified files.Impact
health-check.shmonitoring effectivenessNext Steps (Future PRs)
--strictmode in CI to enforce healthchecksRelated